Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add client wrapper for MongoDB Atlas support #5386

Merged
merged 6 commits into from
Oct 25, 2021

Conversation

zenmoto
Copy link
Contributor

@zenmoto zenmoto commented Sep 22, 2021

Description:

Adds pagination, error handling, will be extended to manage metric
transformation.

@zenmoto zenmoto requested review from a team and codeboten September 22, 2021 20:58
@zenmoto zenmoto force-pushed the mongodbatlas_client_wrapper branch 2 times, most recently from b385ed5 to 87c30de Compare September 28, 2021 18:13
receiver/mongodbatlasreceiver/receiver.go Outdated Show resolved Hide resolved
receiver/mongodbatlasreceiver/receiver.go Outdated Show resolved Hide resolved
receiver/mongodbatlasreceiver/receiver.go Outdated Show resolved Hide resolved
receiver/mongodbatlasreceiver/receiver.go Outdated Show resolved Hide resolved
receiver/mongodbatlasreceiver/receiver.go Outdated Show resolved Hide resolved
@tigrannajaryan
Copy link
Member

Please resolve the conflicts.

@zenmoto zenmoto force-pushed the mongodbatlas_client_wrapper branch 3 times, most recently from 82146e3 to be380cd Compare October 11, 2021 15:58
@zenmoto
Copy link
Contributor Author

zenmoto commented Oct 12, 2021

@jpkrohling I think I addressed your concerns- is this looking right to you?

@jpkrohling
Copy link
Member

I'll review this again tomorrow morning, sorry for the delay.

Copy link
Member

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just please confirm that the print in the stub func at the mongodb_atlas_client should still be like it is.

Copy link
Member

@bogdandrutu bogdandrutu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase since the core receiver changed to use scraperhelper.

Adds pagination, error handling, will be extended to manage metric
transformation.
MongoDB Atlas wrapper returns errors rather than logging
change resource attributes to use conventions
modify error handling
@zenmoto zenmoto force-pushed the mongodbatlas_client_wrapper branch from be380cd to d09dc67 Compare October 21, 2021 00:17
@zenmoto
Copy link
Contributor Author

zenmoto commented Oct 21, 2021

Rebased on scraperhelper updates from main, will defer adding logging support until a future date after discussing with @bogdandrutu.

@tigrannajaryan tigrannajaryan changed the title client wrapper for MongoDB Atlas support Add client wrapper for MongoDB Atlas support Oct 22, 2021
Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
@bogdandrutu do you have other comments on scraper or good to merge?

for _, org := range s.client.Organizations(ctx) {
// Metrics collection starts here
s.log.Debug("fetch resources for MongoDB Organization", zap.String("org", org.Name))
func (s *receiver) poll(ctx context.Context, start string, end string, resolution string) (pdata.Metrics, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: large func, difficult to read. Consider refactoring to smaller bit in a future PR.

@bogdandrutu bogdandrutu merged commit 4e2bc90 into open-telemetry:main Oct 25, 2021
hex1848 pushed a commit to hex1848/opentelemetry-collector-contrib that referenced this pull request Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants